[SG-4885] feat(platform): tirith platform check — a pre-plan policy step, no platform changes, plus the reference docs and a site that publishes - #272

Runs an organization's policies against a plan, state or arbitrary JSON
document from CI or a laptop: masks the document locally, packs it with the
terraform source into an archive, uploads it, creates a StackGuardian run, polls
it and reports the verdict as JSON and/or markdown.

This moves the StackGuardian protocol out of the GitHub Action, where it was
GitHub-only, untestable off a runner, and unavailable to anyone driving the
platform from GitLab or a Makefile. No new runtime dependencies -- the whole
thing is stdlib urllib, so a runner needs nothing beyond tirith itself.

Subcommands are dispatched before the flat parser sees anything. argparse cannot
express an optional subcommand alongside options like `-policy-path`, and the
local-evaluation surface is a contract that test_output_compatibility.py asserts
byte-for-byte. Also fixes cli.main(args=...), which was ignored because
parse_args() was called with no argument.

Two bugs found while writing this:

  * APPROVAL_REQUIRED was missing from the poller's terminal statuses. It is a
    resting state, so a run that reached it spun until the timeout and was then
    reported as a tool failure -- an outage, rather than a finished evaluation
    waiting on a human. It now yields an `approval-required` verdict.
  * A file named state.json in the working directory was packed raw.
    `terraform state pull > state.json` is the documented way to produce one, so
    it routinely sits there unmasked, and it shipped in full beside the masked
    copy. plan.json / state.json / infracost.json are now always written by
    pack() from an already-masked object and never copied from the source tree.

Exit codes: 0 clean, 3 for a policy failure under --fail-on-error, 1 for an
unreachable platform or a run that produced no verdict -- the last regardless of
the flag, because a run with no verdict must never look like a pass.

…c tfstate.json

sensitive_attributes is a list of PATHS -- each entry is itself a list of steps:

    [[{"type": "get_attr", "value": "content_base64"}],
     [{"type": "get_attr", "value": "content"}]]

The code read only the flat forms, so on real state every entry was skipped: a
list is neither a dict nor a string. Nothing in a resource's attributes was
masked at all. The unit test passed because its fixture invented the flat shape;
verified now against `terraform state pull` output for a local_sensitive_file,
which is where the real shape came from.

Paths can also descend through nested objects and list indices, so the masker
walks them rather than assuming a single key, and deep-copies so the caller's
document is not mutated underneath it.

Renames the archive's state document from state.json to tfstate.json, matching
the TfStateCleaned fact it feeds and the name the terraform step already uses
for state. No collision: the archive unpacks into the user directory, while
managed state lives at the artifacts root, and policy-only forces
managedTerraformState off.

A rule result of APPROVAL_REQUIRED means its author wrote
`onFail: APPROVAL_REQUIRED`. The policy-only step records that without pausing
the run -- deliberately, since exit 11 would leave the poller spinning -- so the
run comes back COMPLETED and only the counts carry the intent.

Folding it into `warned` was wrong. `warned` maps to a `neutral` check, which
SATISFIES a required status check, so a policy demanding human sign-off silently
did not block. Ranked above `warned` it produces the `approval-required` verdict,
which the action maps to `action_required` -- honouring the author's intent
without implementing the approval workflow, which is out of scope here.

Caught by a live run against a real APPROVAL_REQUIRED policy: the rule reported
correctly and the verdict said `warned`, so the code handling
`approval-required` was unreachable from this path.

urlencode stringifies None to the literal "None", and the endpoint treats any non-empty
folder as a subfolder -- so the archive landed at .../artifacts/None/__sg.<sha>-<tag>.tar.gz.
Two consequences, both silent: a bogus None/ directory in the workflow's artifact prefix, and
a nested key that the post-run delete could not address, so cleanup no-opped on a 404 and the
archive persisted anyway.

Caught on a live QA run. The folder is now sent only when set; the archive passes none, which
is what puts it at the artifacts root where it can be deleted.

…d show cost in the comment

Infracost and Checkov read `planned_values` and nothing else. The masker drops terraform's
copy -- correctly, because it mirrors every value with NO sensitivity markers, so masking
`resource_changes` leaves the same secret in plaintext there, and a real plan leaked a
`local_sensitive_file` body through exactly that path.

The consequence was that both tools returned a clean, empty and entirely wrong answer.
Measured against infracost 0.10.27 with a real API key, same binary, same plan, differing only
by this section:

    with planned_values     totalMonthlyCost 39.8   1 priced resource
    without (what we ship)  totalMonthlyCost 0      0 priced resources

So the estimate was never a key problem. QA's image key works -- the last run returned
well-formed infracost JSON with no error, just nothing in it.

redact_plan now rebuilds `planned_values` from the *masked* `resource_changes`, after
_mask_by_marker has run. Same data, same shape, no unmarked copy. Only `after`, and only for
resources that will exist: a destroy has no planned value. Module resources are grouped under
`child_modules`; verified that flat and nested forms price identically, and both tools address
resources by the full `address`, which already encodes the module path.

The pull-request comment now carries a cost line, with the delta from the change when infracost
supplies one. Rendered even at zero or on failure, because silence is indistinguishable from
"this change costs nothing" -- very different things to tell a reviewer. It sits outside the
truncation path, so a wall of findings cannot push it out of the comment. Also surfaced as
`monthly_cost` in --output-json for a caller aggregating several units.

client.get_run_facts replaces the narrower get_policy_results as the fetch: the document
carries the verdict and the cost, and embeds the whole plan, so fetching it twice is worth
avoiding. get_policy_results stays as a thin accessor.

196 tests pass, 17 new -- including that the rebuilt section carries __SG_REDACTED__ rather
than the secret, and that terraform's original copy is replaced rather than merged.

A Checkov policy rendered as `❌ best-practices › Policy-Rule-1` with an entirely blank
<details> body -- twelve real findings (EC2 detailed monitoring, EBS encryption, IMDSv1, S3
KMS encryption) reduced to nothing, in the one place a reviewer looks. The verdict was right;
the reasons were invisible.

_extract_detail only understood tirith's shape: a list under `result`, each carrying `message`
and `meta.address`. Checkov entries are `{"description", "keys"}`, so every loop found nothing
and appended nothing. Both shapes now render.

`keys` are reduced to the resource address: Checkov reports `<type>.<name>.<attribute path>`
and the path can be arbitrarily deep, so
`aws_s3_bucket.data.rule.apply_server_side_encryption_by_default.sse_algorithm` becomes
`aws_s3_bucket.data`. The suffix is what the check inspected; the address is what a reviewer
navigates by, and reducing it also collapses several keys on one resource to a single entry.

Tests use the exact payload from QA run iqkxb26uzi1n rather than an invented fixture -- a
fixture is what let this through, since the renderer was only ever exercised against the shape
it already understood. Malformed keys are parametrized, and two tests pin that the tirith
shape and the engine-error path still work.

Also adds CHANGELOG_2026-08-05.md and updates the roadmap: the facts table now reflects that
PolicyEvalResults comes from the run facts rather than a per-run artifact, that Infracost is
written on every run, and that TfStateCleaned is deliberately not written by tirith-check.

…e no longer true

Updated against what is now verified on QA rather than what was true when it was written:

- TfStateCleaned moves from ⚠️ "deliberately not written" to ✅. A post-apply check now
  updates the workflow's Resources view. The reasoning that kept it out was half right: the
  shape mismatch was real and is what the conversion fixes; the workflow-scoped pointer is the
  *intent* for a post-apply check, not a hazard.
- Infracost moves from ⚪ "not exercised" to ✅ generated on every run.
- The archive is now flat and deleted after the run, so the "where it lands" row said something
  that stopped being true.
- A new section records the two-phase pipeline with the facts each phase writes, and why a
  policy with no document on one pass reports WARN.

Three corrections rather than additions:

- "TfStateCleaned and TfPlan are unreachable" was the old symptom of the wfrunfacts bug. Both
  are reachable; the bug is that wfrunfacts 404s on shared-ec2, and its scope is narrower than
  first described -- external.py was never affected, which is why the E2E kept working after
  the fixes were reverted out of this batch.
- The Infracost `$0` finding is added to the ship-blocking table with the evidence that
  isolates it to the image's key: the same plan prices at $35.99 locally, and an invalid key
  reproduces QA's output exactly while a missing key errors loudly.
- Residual `policy-only` references renamed.

Also adds CHANGELOG_2026-08-05.md: everything that changed today, each item linked to the run
that proves it.

The archive is the source that produced the findings, and another system reads it to generate
autofixes. Deleting it after the run removed the only copy of what was actually evaluated.

Retaining it is safe for the runs themselves: the `__sg.` prefix keeps it out of the per-run
artifact sync, so it never lands in a later run's working directory -- which was the problem
worth solving. It is not free, and the code says so: nothing prunes this prefix, so it is one
object per commit and tag, kept indefinitely, and it wants an S3 lifecycle rule.

No fact is written to point at it, because the pointer already exists. The key is on the run
record as RuntimeParameters.terraformProjectZip, verified on a live QA run, so a consumer
holding only a run id can reach the bundle with no platform change and nothing duplicated:

  GET .../wfruns/<id>/                     -> RuntimeParameters.terraformProjectZip
  GET .../wfs/<wf>/get_artifact/?artifactPath=<basename>  -> the bytes
  GET .../wfruns/<id>/wfrunfacts/default/  -> PolicyEvalResults

The plan called for recording the key in SGCustomWorkflowRunFacts. That is dropped: it would
copy data already on the record into a second place that can disagree with it, and the step
cannot see terraformProjectZip anyway -- only wfStepInputData reaches the container, so it
would have needed a core change to carry a value the consumer can already read.

`archive_key` is added to --output-json for a caller that has the result document in hand.
`client.delete_artifact` stays: it is tested, and a retention sweep will want it.

Note for consumers: the archive holds the masked plan and, only when `source-dir` is set, the
terraform source. The default ships no source, so autofix callers must set it or they will get
a bundle with nothing to fix.

A state document uploaded with --state-path was only reachable by unpacking the
run's archive, so it appeared in neither the State view nor the artifacts list.
It is now also written to `artifacts/tfstate.json`.

That name is canonical rather than chosen: the managed-state backend writes it,
state locking keys on the literal basename, and the state-backends listing
special-cases it. So no new API endpoint is needed either -- `tfstate_upload_url`
and `file_upload_url` are the same view, and its default filename is already
`tfstate.json`.

Unlike the archive this object is deliberately NOT `__sg.`-prefixed: it is meant
to be seen.

Two guards, because the same property that makes the name useful makes it
dangerous:

  * If the workflow manages its own terraform state, the upload is skipped. For
    such a workflow that object IS the live state, and writing a masked document
    over it is data loss. An unreadable answer counts as managed -- absent is not
    the same as false, and not being able to tell is not a reason to overwrite.
  * The log says the published copy is masked and cannot be used to run
    terraform. A file at the canonical state key full of __SG_REDACTED__ is a
    footgun for whoever downloads it next.

The upload is best-effort: a run whose policies evaluated correctly must not go
red because a convenience copy could not be written.

`upload_archive` becomes `upload_file` with a content type, since a JSON state
document cannot be sent with the archive's `application/gzip` -- S3 signs the
content type into the URL. Its body parameter is named `content`: calling it
`payload` shadowed the response variable and sent the JSON response to S3 in
place of the file, which an existing test caught.

376 tests pass, 10 new.

The terraform source is packed by default, so an exclusion that does not fire --
a committed vendor directory, a build output tree -- turned a working policy
check into a failed run. `archive.pack` raises above 100 MB gzipped and nothing
caught it: the pack call sat outside run_check's try block.

That trade is the wrong way round. The verdict gates the merge; the source is a
convenience for whatever reads the bundle afterwards. So an oversized archive now
degrades to documents-only and says so, loudly, instead of taking the check down
with it.

Only when a source tree was actually requested. Already documents-only and still
over the limit means the *documents* are too big and there is nothing left to
drop, so that stays fatal -- uploading an archive with no documents is not a
check at all.

The result document records `source_packed` and `source_skipped_reason`, because
"the bundle has no code" and "no code was wanted" have to be distinguishable by
a consumer that only has the document. The GitHub annotation is raised by the
action, not here: this module stays VCS-agnostic so a GitLab or Jenkins caller
reuses it unchanged.

Two things fixed while in here:

  * The size message reported anything under a megabyte as "0 MB, over the 0 MB
    limit" from integer division. It is now human-readable, which matters because
    the message is surfaced on a pull request.
  * MAX_ARCHIVE_BYTES is overridable via TIRITH_MAX_ARCHIVE_BYTES. With the
    source packed by default, the only other lever was dropping it entirely, so a
    large monorepo that genuinely needs to ship its code had nowhere to go. A
    non-numeric value is ignored rather than failing a run.

214 platform tests pass, 4 new.

The pull-request comment is edited in place across runs, so it shows the latest
verdict and nothing else. Without naming the revision, a reader has no way to
tell whether what they are looking at is about the head of the branch or about a
push from an hour ago -- and the more confident the verdict reads, the worse that
ambiguity is.

`render_markdown` takes an optional `commit`, rendered as a subline under the
headline. Doing it here rather than letting the caller append means the check-run
summary and the job summary get it too, from one place.

Abbreviated to seven characters, as git does -- but only when it actually looks
like a hex sha. A tag or branch name is passed through whole: truncating one
would produce something that looks like a sha and is not.

`check.py` threads the existing `opts.sha`, which already feeds the archive name,
so nothing new has to be plumbed in.

219 platform tests pass, 5 new.

Two clean-ups, both of my own making.

`git add -A` in cbc397c swept in eighteen untracked files from the working tree
-- an unrelated ansible/jq/jmespath exploration under tests/providers/json/ --
and 9d0cc81 did the same with two of my session notes. None of it belongs to
SG-4885, and test_ansible_best_practices_jq.py fails ("operation_type: jq_query
is not supported"), which is what turned this PR's unittest and coverage jobs
red.

Removed with `git rm --cached`: every file stays on disk exactly as it was,
untracked and unchanged.

Then black over the files this branch actually owns. Measured on clean checkouts
rather than the working tree, because the working tree is full of untracked
files that skew it: main already fails black on 14 files, so the lint job was red
before this branch existed. This branch was adding nine more; those are fixed.
The pre-existing fourteen are deliberately left alone -- reformatting them is a
repo-wide decision, not this PR's, and it would bury the diff.

385 tests pass.

…e files I touched

**The 3.8 and 3.9 unittest jobs were failing on a test I wrote**, and for a reason the test
itself created. `test_the_usage_block_is_the_real_help_output` compared the README's Usage
block byte for byte against `tirith --help` -- but argparse renamed its section header from
"optional arguments:" to "options:" in 3.10, so a block generated on any one interpreter
cannot match on the other half of the matrix. It passed on 3.10-3.12 and failed on 3.8-3.9,
which is the worst shape for a guard test: it looks like it works.

Now compares the *set of option strings* in both directions -- accepted but undocumented,
documented but not accepted. That is the thing the test existed to catch (a `-var-path` that
shipped undocumented), and it does not depend on how argparse decides to lay out a heading.
Verified against a simulated 3.8 header.

**Black.** Four files, all touched by this PR: core.py, platform/check.py,
tests/platform/test_client.py, tests/platform/test_report.py. Formatted only those --
`origin/main` itself fails Black on 14 files because CI pins `psf/black@stable` (unpinned,
so whatever is newest) against a tree formatted by an older release, and reformatting the
other ten here would bury this PR in an unrelated diff. That drift is worth fixing on main
by pinning the version, separately.

The Black job was red on this PR and on `main` alike, on 14 files nobody had touched. Cause:
`psf/black@stable` resolves to whatever Black is newest when the job runs, so a Black
release reformats the world and every open branch goes red with nothing in the repository
having changed. main last passed this job in November 2025 and fails it today.

Pinned to 25.1.0, which is the release the tree is actually formatted for -- verified by
running it against `origin/main`, which comes back clean, and against 24.10.0, which also
does. Bumping it should be a deliberate commit that reformats, not a surprise from
upstream.

Reverted my earlier attempt to fix this by formatting four files with 26.5.1: that was
chasing the newest release rather than the pinned one, and would have left the tree
inconsistent with the other 85 files. The tree is now clean under 25.1.0 end to end.

archive.pack raises ArchiveError both for an oversized archive and for a source directory
that does not exist, and pack_documents' degrade path only knew about the first. So a
typo'd --source-dir was reported as "the tree was too large", the code was dropped, and
the run completed -- a check that passed having evaluated no source at all, with the
bundle's own metadata.json stating the wrong reason for its absence.

Checked before packing, where the two are still distinguishable. A missing directory is a
user error and should stop the run; only a genuinely oversized tree degrades.

…st F2)

A secret used in a resource tag was masked at `tags.Password` and uploaded in **cleartext**
at `tags_all.Password`. Both state shapes leaked, and the bundle is retained indefinitely,
so the plaintext outlived the run.

`redact_plan` has swept for this since the equivalent plan leak was found: it collects the
plaintext of every marked-sensitive value and replaces that value everywhere in the
document, precisely because a provider writes computed *mirrors* of an attribute carrying
the same secret with no sensitivity marker of their own. `tags_all` is the confirmed case
-- terraform does not propagate sensitivity into values it computes for you.

`redact_state` masked by marker and returned. Same hole, worse place: state carries every
attribute of every resource, not just what a plan surfaces. Found by a penetration test
(F2, Medium), reproduced code-level, and reproduced again here against both shapes before
and after.

It now collects at all four masking sites and sweeps the whole document on the way out:

  show -json resources    _collect_sensitive_values against `sensitive_values` -- the same
                          marker convention as a plan, so this is a direct reuse
  raw instances           read the plaintext at each resolved `sensitive_attributes` path
  outputs, both shapes    collect `output.value` before it is replaced

The raw shape needed a path *reader*: `_mask_attribute_path` walks to a leaf and assigns,
with nothing to read the same path back. `_read_attribute_path` mirrors that walk exactly,
because the two have to agree on what a path means or the sweep collects a different value
from the one that was masked. It reads from the untouched original rather than the copy
being masked -- after the first path is masked the copy holds the sentinel there, and
sweeping for that would do nothing.

Outputs are worth noting separately: an output's plaintext was discarded when it was
masked, so nothing else knew it was a secret, and the same value in an ordinary attribute
stayed in cleartext.

Six tests, in both shapes, all failing before this change. Including the two bounds that
keep a value-based sweep honest: `region` survives, and a value under
MIN_SWEPT_SECRET_LENGTH is not swept, so masking `Env: dev` does not redact every `dev` in
the document.

…pentest F1)

Every attacker-influenced string in the report was interpolated raw or wrapped in a single
backtick, and a backtick *in the value* closes that span so the remainder renders as
markdown and HTML. A pull-request author controls the terraform a plan is built from, so
they controlled the report a reviewer reads: the pen test produced a fake "all policies
passed" banner and a link whose text said app.stackguardian.io and whose href pointed
elsewhere. Reviewers get that by email too. The verdict and the exit code were never
affected -- this is report corruption, not a gate bypass.

The report named three sinks. There are eight, and two it did not mention:
the infracost `currency` and the `str(monthly)` fallback go inside `<sub>`, and the run URL
goes inside an `href`. Also `rule_name` was the only field with no wrapping *at all*, in a
table cell and inside `<summary>` -- the strongest sink in the file.

Two primitives, because the sinks are in two different languages:

`_code()` for markdown contexts. A code span whose fence is one backtick longer than the
longest run inside the value, per CommonMark, so it cannot be closed from within. Inert by
construction rather than by enumerating dangerous characters. Escaping was the alternative
and is worse here: the engine deliberately puts backticks in its own messages
(`json_format_value` wraps every compared value), so escaping them puts visible backslashes
through every finding, and it holds only while the character list stays complete. Newlines
collapse to a space, and pipes become `\|` in table cells -- GFM's documented escape and
the one that works inside a span.

`_html()` for values going into `<summary>`, `<code>`, `<sub>` and the `href`. A code span
is wrong there: GFM does not reliably render markdown inside inline HTML. It also escapes
backticks, which `html.escape` does not -- they cannot close a span in the summary because
there is none, but an odd one OPENS one that swallows the markdown after it, so
``cost-control` `` still distorted the report with the tags already neutralised. Found while
verifying, not while designing.

`check.py` also now quotes org, workflow group and workflow id into the run URL, the way
client.py already quotes the same three on every API path.

Nine tests, all failing before this change, asserting against markdown **rendered by a
CommonMark parser** rather than against the source -- the payload is still in the source by
design, inside a span where it is inert, so a substring check on the source proves nothing.
That was the mistake I made first while verifying this.

One visible change beyond the fix: `rule_name` now renders in a code span like `policy_id`
already did, so ordinary rows gain backticks around the rule. Consistent, but not
byte-identical to before -- I had assumed benign output would be unchanged and a test proved
otherwise.

The rename to `remote` was made on the argument that "platform check" can read as *a check
of the platform*, which is what `--platform` means in most tools. Reverted: the vagueness
is minor next to the cost of having two names in circulation, and the concern that
prompted it was really that the open-source surface could not gate at all -- which
`--fail-on-error` fixed, and no rename would have.

No alias in either direction. Nothing is released -- py-tirith is not on PyPI and the
action pins a branch -- so there was never a caller to keep working, which is what made
both this and the original rename cheap.

`docs/remote-check.md` moves back with git mv so the history follows, and both embedded
`--help` blocks are regenerated under the restored name. The dispatch test now pins the
outcome rather than the direction: exactly one subcommand name, and `remote` is not
quietly still accepted.

Kept from the rename work, because neither depended on the name: the one-column
continuation fix in `--help`, and `status.py` no longer naming a subcommand at all in its
exit-code comment.

Pages is enabled on this repo but has never served anything -- the gh-pages
branch contains only .nojekyll, so https://stackguardian.github.io/tirith/
returns 404. This is a stopgap until the designer rebuilds it next week.

Deliberately plain: one hand-written HTML file, no generator, no build step and
no JavaScript, because the repo has no site tooling and a placeholder is a bad
reason to introduce some. Every prose block sits in its own commented <section>
so the copy can be lifted out without reading the markup.

All copy is derived from README.md rather than newly written, so there is one
source of truth to keep correct. Claims are limited to what the code actually
does: no OPA (nothing in src/ references it), and no approvals, since a policy
asking for approval warns rather than gates.

Serving this needs a one-time Pages settings change -- source from a branch,
folder /docs, replacing the empty gh-pages branch. docs/.nojekyll keeps Jekyll
off the existing .md and .gif files in that folder.

This reverts commit b5a7a5f.